Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPEDGE-1192: feat: initial arbiter node addition #1731

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eggfoobar
Copy link

added support for creating install for clusters with arbiter nodes

In 4.19 of OCP, we will be supporting a deployment with Two Node Openshift with Arbiter on the baremetal platform, this PR adds the arbiter configuration to the metal scripts to help deploy and test arbiter topologies for baremetal.

For more details see EP: openshift/enhancements#1674

added support for creating install for clusters with arbiter nodes

Signed-off-by: ehila <[email protected]>
@openshift-ci openshift-ci bot requested review from bfournie and rwsu January 27, 2025 18:19
Copy link

openshift-ci bot commented Jan 27, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign bfournie for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 27, 2025
Copy link

openshift-ci bot commented Jan 27, 2025

Hi @eggfoobar. Thanks for your PR.

I'm waiting for a openshift-metal3 member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@eggfoobar
Copy link
Author

eggfoobar commented Jan 27, 2025

/hold

Currently running int an odd issue where I'm not sure if it's a problem with the devscript or not. But the arbiter node takes around 10minutes to join the cluster because the inspecting phase takes too long. @zaneb Would you be able to know why that is?

I did create this PR in baremetal for the provisioning controller, but don't if I'm missing something else here. openshift/cluster-baremetal-operator#460

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2025
@iurygregory
Copy link
Contributor

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 27, 2025
Copy link

openshift-ci bot commented Jan 27, 2025

@eggfoobar: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agent-5control-ipv4 0e2ea49 link false /test e2e-agent-5control-ipv4
ci/prow/e2e-agent-ha-dualstack 0e2ea49 link false /test e2e-agent-ha-dualstack
ci/prow/e2e-agent-4control-ipv4 0e2ea49 link false /test e2e-agent-4control-ipv4
ci/prow/e2e-agent-compact-ipv4 0e2ea49 link true /test e2e-agent-compact-ipv4
ci/prow/e2e-metal-ipi-ovn-dualstack 0e2ea49 link false /test e2e-metal-ipi-ovn-dualstack
ci/prow/e2e-agent-sno-ipv6 0e2ea49 link false /test e2e-agent-sno-ipv6
ci/prow/e2e-metal-ipi-bm-bond 0e2ea49 link false /test e2e-metal-ipi-bm-bond
ci/prow/e2e-metal-ipi-ovn-ipv6 0e2ea49 link true /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-metal-ipi-serial-ovn-ipv6 0e2ea49 link false /test e2e-metal-ipi-serial-ovn-ipv6
ci/prow/e2e-metal-ipi-serial-ipv4 0e2ea49 link true /test e2e-metal-ipi-serial-ipv4
ci/prow/e2e-metal-ipi-virtualmedia 0e2ea49 link false /test e2e-metal-ipi-virtualmedia
ci/prow/images 0e2ea49 link true /test images
ci/prow/e2e-metal-ipi-bm 0e2ea49 link true /test e2e-metal-ipi-bm

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@zaneb
Copy link
Member

zaneb commented Jan 28, 2025

No idea about the inspection thing.

Creating a new node type and duplicating all of the config for it seems like a... high-maintenance way to do this. Couldn't we treat the arbiter node as a control-plane node everywhere except when generating the install config?

@@ -136,6 +136,7 @@ ansible-playbook \
-e "baremetal_network_name=${BAREMETAL_NETWORK_NAME}" \
-e "working_dir=$WORKING_DIR" \
-e "num_masters=$NUM_MASTERS" \
-e "num_arbiters=$NUM_ARBITERS" \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The playbook we're using here comes from https://github.com/metal3-io/metal3-dev-env/ which is an upstream project and thus does not (and can not) know about OCP-specific node types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants